Curve Analysis class #46

nkanazawa1989 · 2021-05-11T14:28:16Z

Summary

This PR adds CurveAnalysis class that provides basic functionalities to extract curve data and perform proper error handling for subclasses.

Details and comments

Based on this base class, we can easily implement several fitting subclasses. Data extraction and error handling is nicely performed behind the scene. This class overrides BaseAnalysis._run_analysis and provides several function-wise private methods that subclasses can override. For example _run_fitting, _create_figure, _data_processing, and _post_processing. Internal data representation CurveEntry (named tuple) is introduced to manage curve data.

To define new analysis, basically you can override base class attributes.

For example, T1 experiment fit may be written as

class T1Analysis(CurveAnalysis):
    __x_key__ = "delay"
    __fit_funcs__ = [library.exponential]
    __param_names__ = ["a", "tau", "b"]

and IRB fit may be written as

class IRBAnalysis(CurveAnalysis):
    __x_key__ = "ncliffs"
    __series__ = [
        SeriesDef(
            name="standard_rb",
            param_names=["a", "alpha_std", "b"],
            fit_func_index=0,
            filter_kwargs={"interleaved": False}
        ),
        SeriesDef(
            name="interleaved_rb",
            param_names=["a", "alpha_int", "b"],
            fit_func_index=0,
            filter_kwargs={"interleaved": True}
        )
    ]
    __fit_funcs__ = [library.exponential]
    __param_names__ = ["a", "alpha_std", "alpha_int", "b"]

As you can see subclasses don't need to write their own code. This will give us two benefits:

This drastically simplifies unittests and maintenance overhead (you only need to write unittest for method that you override).
We can easily serialize the analysis class (if method is not overridden).

Hint for review

Details are written in the CurveAnalysis class docstring.
Actual usages are shown in unittest.
Flow of fit operation
1. _run_analysis is called
2. _run_analysis calls _extract_curves.
3. _extract_curves calls _data_processing (so that subclass can override the logic).
4. _extract_curves generates CurveEntrys and return them.
5. _run_analysis calls _run_fitting (so that subclass can override the logic, i.e. initial guess/weights/bounds...)
6. _run_fitting calls _series_curve_fit
7. _series_curve_fit performs a linearized multi-objective non-linear least squares fit and returns AnalysisResults.
8. _run_analysis calls _post_processing followed by _create_figure.
9. _run_analysis returns analysis results and figures

- bug fix - lint

yaelbh

Overall it looks very good. My comments are mostly questions/hesitations and typos.

qiskit_experiments/analysis/curve_analysis.py

yaelbh · 2021-05-12T11:23:17Z

qiskit_experiments/analysis/curve_analysis.py

+        Returns:
+            New AnalysisResult instance containing the result of post analysis.
+        """
+        return analysis_result


Also here, not sure whether to return a dictionary or an AnalysisResultV1 object

qiskit_experiments/analysis/curve_analysis.py

Co-authored-by: Yael Ben-Haim <[email protected]>

eggerdj

I left some ideas on how to better deal with the data processing.

qiskit_experiments/analysis/curve_analysis.py

test/analysis/test_curve_fit.py

Co-authored-by: Daniel Egger <[email protected]>

qiskit_experiments/analysis/curve_analysis.py

gadial

Most of the PR is really excellent and will save some of the current complexity in RB; however, I'm not sure if it'll be possible to use RB with it without some further additions (in particular the handling of p0).

- add default data processor - add data processor calibraiton - integrate run_fit and series fit - add data pre-processing (i.e. RB needs to take mean) - add fitter setup for initial guess and other options

…kit-experiments into feature/curve_analysis

…ure/curve_analysis

nkanazawa1989 · 2021-05-13T08:38:17Z

Thanks for reviewing! @eggerdj @gadial @yaelbh
I updated the PR and I think it's ready for the second review.

eggerdj

Thanks for adding the training logic. I still have a few questions and comments related to it.

qiskit_experiments/analysis/curve_analysis.py

eggerdj · 2021-05-13T15:19:38Z

qiskit_experiments/analysis/curve_analysis.py

+    return AnalysisResult(result)
+
+
+def level2_probability(data: Dict[str, Any], outcome: Optional[str] = None) -> Tuple[float, float]:


level2_probability can be removed, see my comment above.

qiskit_experiments/analysis/curve_analysis.py

qiskit_experiments/analysis/fit_functions.py

…kit-experiments into feature/curve_analysis

eggerdj

Looks mostly good. Still a few comments and questions.

qiskit_experiments/characterization/qubit_spectroscopy.py

qiskit_experiments/analysis/curve_analysis.py

qiskit_experiments/characterization/qubit_spectroscopy.py

qiskit_experiments/analysis/curve_analysis.py

qiskit_experiments/analysis/curve_fitting.py

qiskit_experiments/analysis/curve_analysis.py

Co-authored-by: Daniel Egger <[email protected]>

- add y normalization

qiskit_experiments/analysis/curve_analysis.py

Co-authored-by: Daniel Egger <[email protected]>

eggerdj

LGTM

* wip curve fit analysis * wip curve fit analysis * curve_fit complete * - unittest - bug fix - lint * black * removed redundant code * fix unittest * fix docstring * fix docstring * wording fix Co-authored-by: Yael Ben-Haim <[email protected]> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <[email protected]> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <[email protected]> * Feedback from eggerdj - add default data processor - add data processor calibraiton - integrate run_fit and series fit - add data pre-processing (i.e. RB needs to take mean) - add fitter setup for initial guess and other options * readd 52c99ea and 0420d1d * add fit functions * lint * review by eggerdj r2 - change dp calibration logic - add gaussian function * add data processor keys in metadata add data processor keys in metadata * conform to qiskit-community#41 * feedback from chris1 - more general docstring - add default parameter to fit funcs * feedback from chris2 - parameter dict handling * feedback from chris3 - import path * lint * add fit option validation * add unittest and integration test * black & lint * simplify the analysis class * misc * add default figure generation * lint * add default value * fix docstring typo * remove outcome from default processing options * black and lint * add processor training check * fix pre processing routine * update rb analysis as an example - add num_qubits to curve analysis - add label generation to curve analysis * post process error handling when fit failed * add axis label * update axis formatting * more integration with new options * update irb * move class attributes to options * review comment from Chris * fix bug causes list indices must be ... * Analysis result formatting * update error docstring * lint * bug fix - reorder args of protected methods (series comes before xvals) - make analysis result list - relevant test fix - add filter kwargs to interleaved analysis * lint * fix spect analysis * fix composite analysis to use instance's analysis option * adjust curve figure appearance * black * update data and option handling * - private -> protected member - param list generation and validation in new method - all nan sigma handling - zero sigma handling in curve fitter - add data extraction utils - allow bounds = None - * update spectroscopy * black & lint * rerun rb notebook * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <[email protected]> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <[email protected]> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <[email protected]> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <[email protected]> * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <[email protected]> * - more docstring - add y normalization * black * add TODO comment * add analysis class information to result data * fix unittest * update spect analysis docstring * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <[email protected]> * rb analysis class docstring * format docstring * Update qiskit_experiments/analysis/curve_analysis.py Co-authored-by: Daniel Egger <[email protected]> Co-authored-by: Yael Ben-Haim <[email protected]> Co-authored-by: Daniel Egger <[email protected]> Co-authored-by: Christopher J. Wood <[email protected]>

nkanazawa1989 added 4 commits May 11, 2021 02:27

wip curve fit analysis

ab027ce

wip curve fit analysis

6b35027

curve_fit complete

d0e171f

- unittest

b34da0d

- bug fix - lint

nkanazawa1989 requested review from chriseclectic, yaelbh, eggerdj and gadial May 11, 2021 14:28

nkanazawa1989 added 5 commits May 11, 2021 23:38

black

94859af

removed redundant code

37d3c1d

fix unittest

2f49726

fix docstring

78fbd7a

fix docstring

89aad76

yaelbh requested changes May 12, 2021

View reviewed changes

wording fix

1639af6

Co-authored-by: Yael Ben-Haim <[email protected]>

eggerdj suggested changes May 12, 2021

View reviewed changes

nkanazawa1989 and others added 2 commits May 13, 2021 11:22

Update qiskit_experiments/analysis/curve_analysis.py

52c99ea

Co-authored-by: Daniel Egger <[email protected]>

Update qiskit_experiments/analysis/curve_analysis.py

0420d1d

Co-authored-by: Daniel Egger <[email protected]>

gadial reviewed May 13, 2021

View reviewed changes

qiskit_experiments/analysis/curve_analysis.py Outdated Show resolved Hide resolved

gadial approved these changes May 13, 2021

View reviewed changes

nkanazawa1989 added 6 commits May 13, 2021 16:00

Feedback from eggerdj

a3dd099

- add default data processor - add data processor calibraiton - integrate run_fit and series fit - add data pre-processing (i.e. RB needs to take mean) - add fitter setup for initial guess and other options

Merge branch 'feature/curve_analysis' of github.com:nkanazawa1989/qis…

9b5e631

…kit-experiments into feature/curve_analysis

readd 52c99ea and 0420d1d

1c4913c

Merge branch 'main' of github.com:Qiskit/qiskit-experiments into feat…

d944a29

…ure/curve_analysis

add fit functions

eb29087

lint

c3ced56

yaelbh approved these changes May 13, 2021

View reviewed changes

nkanazawa1989 mentioned this pull request May 13, 2021

[WIP] Mock experiment #49

Closed

3 tasks

eggerdj suggested changes May 13, 2021

View reviewed changes

nkanazawa1989 added 3 commits June 5, 2021 00:31

rerun rb notebook

8c74427

Merge branch 'feature/curve_analysis' of github.com:nkanazawa1989/qis…

c9e4361

…kit-experiments into feature/curve_analysis

Merge branch 'main' into feature/curve_analysis

9bcab0d

eggerdj suggested changes Jun 7, 2021

View reviewed changes

eggerdj mentioned this pull request Jun 7, 2021

Rabi #74

Merged

nkanazawa1989 and others added 10 commits June 7, 2021 21:06

Update qiskit_experiments/analysis/curve_analysis.py

a1be597

Co-authored-by: Daniel Egger <[email protected]>

Update qiskit_experiments/analysis/curve_analysis.py

1bbc199

Co-authored-by: Daniel Egger <[email protected]>

Update qiskit_experiments/analysis/curve_analysis.py

9f294b0

Co-authored-by: Daniel Egger <[email protected]>

Update qiskit_experiments/analysis/curve_analysis.py

8449161

Co-authored-by: Daniel Egger <[email protected]>

Update qiskit_experiments/analysis/curve_analysis.py

0729d6c

Co-authored-by: Daniel Egger <[email protected]>

- more docstring

e3bb022

- add y normalization

black

abe5ea4

add TODO comment

0e9e89f

add analysis class information to result data

bc060d7

fix unittest

03bc71b

eggerdj reviewed Jun 7, 2021

View reviewed changes

qiskit_experiments/analysis/curve_analysis.py Outdated Show resolved Hide resolved

nkanazawa1989 and others added 5 commits June 8, 2021 03:57

update spect analysis docstring

b416c80

Update qiskit_experiments/analysis/curve_analysis.py

aea9e26

Co-authored-by: Daniel Egger <[email protected]>

rb analysis class docstring

8f20f55

format docstring

03dafd4

Update qiskit_experiments/analysis/curve_analysis.py

871bfbb

Co-authored-by: Daniel Egger <[email protected]>

eggerdj approved these changes Jun 8, 2021

View reviewed changes

eggerdj mentioned this pull request Jun 8, 2021

[WIP] Resonator spectroscopy #90

Closed

4 tasks

Merge branch 'main' into feature/curve_analysis

0da07b1

chriseclectic approved these changes Jun 8, 2021

View reviewed changes

eggerdj merged commit be3e21e into qiskit-community:main Jun 8, 2021

yaelbh mentioned this pull request Jun 10, 2021

Update T1 and T2* with the new curve analysis class #98

Closed

coruscating added this to the Release 0.1 milestone Jun 16, 2021

nkanazawa1989 deleted the feature/curve_analysis branch October 27, 2022 06:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Curve Analysis class #46

Curve Analysis class #46

nkanazawa1989 commented May 11, 2021 •

edited

Loading

yaelbh left a comment

yaelbh May 12, 2021

eggerdj left a comment

gadial left a comment

nkanazawa1989 commented May 13, 2021

eggerdj left a comment

eggerdj May 13, 2021 •

edited

Loading

eggerdj left a comment

eggerdj left a comment

		return AnalysisResult(result)


		def level2_probability(data: Dict[str, Any], outcome: Optional[str] = None) -> Tuple[float, float]:

Curve Analysis class #46

Curve Analysis class #46

Conversation

nkanazawa1989 commented May 11, 2021 • edited Loading

Summary

Details and comments

Hint for review

yaelbh left a comment

Choose a reason for hiding this comment

yaelbh May 12, 2021

Choose a reason for hiding this comment

eggerdj left a comment

Choose a reason for hiding this comment

gadial left a comment

Choose a reason for hiding this comment

nkanazawa1989 commented May 13, 2021

eggerdj left a comment

Choose a reason for hiding this comment

eggerdj May 13, 2021 • edited Loading

Choose a reason for hiding this comment

eggerdj left a comment

Choose a reason for hiding this comment

eggerdj left a comment

Choose a reason for hiding this comment

nkanazawa1989 commented May 11, 2021 •

edited

Loading

eggerdj May 13, 2021 •

edited

Loading